fix: rebuild block 0 from the scanned UID when it can't be read - #936
fix: rebuild block 0 from the scanned UID when it can't be read#936cp09x wants to merge 1 commit into
Conversation
|
For reference, how much of this is AI? Also: What do you do if you read a card with a UID full of zeros? Maybe youre reading a existing magic card that has a 0 uid? |
Hi. I used AI to help trace it and write the patch, but it's a real bug I hit (cloning my fob to a gen1a magic card left it with UID 00 00 00 00 and the reader rejected it). ...On the zero UID card: block 0 gets rebuilt from card.uid (from the scann), not the zeroed bytes, so a real 0 UID card stays 0 and a normal card still gets its real UID back. The all zero check is just a proxy for "block 0 wasn't captured." Fair that it's a guess though. Cleaner would be to build block 0 at dump time from the scanned UID/SAK/ATQA (mfClassicGenerateFirstBlock already does this). I can have a look on that way if you want.. |
71da657 to
2fede31
Compare
|
I made some other changes... block 0 now gets rebuilt at dump/save time from the scanned UID/SAK/ATQA (mfClassicGenerateFirstBlock), so nothing keys off an all zero block 0 anymore. A card with an original 0 UID rebuilds to the same 0 UID, and I added a test for exactly that. Kept the write side rebuild as a fallback for dumps saved before this change, since they still carry the correct UID in the uid field. What do you think??? |
|
Well, even then, its all still just a heuristic? Shouldnt we be able to check if block 0 got read successfully (at time of reading) and then pop up a warning or smthng? Blindly overwriting data because of some zeros being in unusual, but possible places seems like a very bad idea and great road to bugs? Also: Am I wrong or does the reconstruction logic have an error with ATQA and SAK handling? (SAK 08 ATQA 004 turns into 88 00 04?) |
|
Yeah fair, you're right. Good catch on the SAK/ATQA too, that builder adds 0x80 to the SAK and doesn't flip the ATQA, so it comes out wrong. |
|
Good idea. Make sure tho that the user is aware of that and has the option to cancel (eg popup). We dont want to accidentally change data without the user knowing. |
2fede31 to
6074c81
Compare
cp09x
changed the title
github-actions
Bot
added
the
translation changes
When a Mifare Classic card is dumped but block 0 (the UID block) can't be read, it was stored as 16 zero bytes. Writing that dump to a magic card copied the zeros and blanked the card's UID, so the clone read back as 00 00 00 00 and readers rejected it. Emulation was unaffected because it uses the uid field directly. Track at read time whether block 0 was actually read (in dumpData) rather than inferring it from zeros. When it wasn't, prompt the user with a confirm dialog before rebuilding block 0 from the scanned UID, so data is never changed silently; a card with a genuine zero UID stays zero. Also fix mfClassicGenerateFirstBlock, which set SAK to sak + 0x80 and did not reverse the ATQA (so 08/0004 became 88 00 04); this also corrects the create and edit card dialogs. Adds builder tests.
6074c81 to
9732268
Compare
|
Done. Now it pops up a dialog when block 0 couldn't be read, with Cancel or Rebuild UID. |
|
Sounds good. will test and review later |
2 participants
Fixes cloning a Mifare Classic card to a magic card ending up with UID
00 00 00 00, which readers reject. Emulation worked fine because it uses the UID field directly, only writing to a card broke.The cause: when block 0 (the UID block) can't be read, it's saved as zeros, and writing those zeros blanks the card's UID.
Now the dump tracks whether block 0 actually read. If it didn't, it asks the user first (a popup with Cancel or Rebuild) before touching block 0, so nothing changes silently. Cancel saves the dump unchanged. A card with a real 0 UID stays 0.
Also fixes
mfClassicGenerateFirstBlock(SAK was set tosak + 0x80and the ATQA wasn't reversed, so08/0004became88 00 04). That corrects the create and edit card dialogs too.